Skip to content

feat(workflows): sandbox tenant mirror, org block and tier exclusion - #128411

Closed
Silthus wants to merge 6 commits into
workflows-sandbox-safety/capsfrom
workflows-sandbox-safety/django
Closed

Silthus wants to merge 6 commits into
workflows-sandbox-safety/capsfrom
workflows-sandbox-safety/django

Conversation

@Silthus

@Silthus Silthus commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Changes

  • Staff block an organization with a SandboxSenderOrganizationBlock row in the Django admin. The provision endpoint then returns 404 for its projects.
  • Django mirrors the sandbox SES tenant's sending status from the tenant-state webhook and the daily sweep, and tells the workers after commit.
  • The sending tier and the sending allowance subtract email_sandbox_sent from email_sent, floored at zero. Bounces and complaints still count.
  • Setup status no longer treats the sandbox sender as a channel the project set up.
  • The tier and allowance queries pick their metric list from the deployment settings, not the team flag.
  • With any sandbox setting empty, every query and check matches master.
  • Once the settings are set, every team's query includes email_sandbox_sent. A team without sandbox sends still gets master's numbers, because that metric is 0 for it.
  • Migrations 0041 and 0042 add two tables. Their foreign keys use db_constraint=False, so no hot table is locked.

How did you test this code?

Test rationale: one mixed-traffic case per metric reader checks that sandbox sends drop out of the volume and allowance, and that an unconfigured deployment queries master's metric list. Tenant tests cover a changed state, an unknown tenant, and an empty tenant name.

  • Not run: the full backend suite.

Release status

  • This change is behind a feature flag and is not available to users

Docs update

  • docs/internal/workflows-email-sending-tiers.md describes the block, the tenant mirror and the tier exclusion.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

Skills: /django-migrations, /writing-tests, /writing-code-comments, /writing-pr-descriptions

More context for AI agents
  • Decisions: the exclusion is per day and per workflow, so a day with email_sandbox_sent but no matching email_sent subtracts nothing.
  • Decisions: until feat(cdp): cap sandbox sends per project and recipient each day #128410's Node metric deploys, email_sandbox_sent is zero and the numbers match master.
  • Decisions: the SES reputation read for team tenants is a refactor with master's call order, so the sandbox mirror can reuse it.
  • Decisions: migrations renumbered after master's 0040.
  • Read closely: own_sends and sent_metric_names in email_sending_tier.py, sync_sandbox_tenant_state in ses_tenant_state.py, providers/ses.py.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane, mixes lanes

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

This PR changes Python or frontend code (frontend/src/lib/components/CyclotronJob/CyclotronJobInputs.tsx, frontend/src/lib/constants.tsx, frontend/src/lib/integrations/IntegrationsList.tsx and 70 more) and Node or Rust code (nodejs/package.json, nodejs/src/cdp/cdp-services.ts, nodejs/src/cdp/config.ts and 14 more) together. In the merge queue, every Node or Rust PR behind it in the same lane then runs the Django and frontend suites too, so the cross-lane check fails. Split it, or add the cross-lane-change label if the halves must land together.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Django migration SQL — 2 new migrations to review

We've detected new migrations on this PR. Review the SQL output for each migration:

products/workflows/backend/migrations/0041_sandbox_sender_tenant_state.py

/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
  return result
2026-11-04T07:50:08.201623Z [error    ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=5863 tid=139757258898304
Traceback (most recent call last):
  File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
    geoip: Optional[GeoIP2] = GeoIP2(cache=8)
                              ~~~~~~^^^^^^^^^
  File "/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 114, in __init__
    raise GeoIP2Exception(
        "Path must be a valid database or directory containing databases."
    )
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
  return (ssh_host,
System check identified some issues:

WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model SandboxSenderTenantState
--
CREATE TABLE "workflows_sandboxsendertenantstate" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "tenant_name" varchar(64) NOT NULL UNIQUE, "sending_status" varchar(32) DEFAULT '' NOT NULL, "synced_at" timestamp with time zone NULL);
CREATE INDEX "workflows_sandboxsendertenantstate_tenant_name_d55f9c1c_like" ON "workflows_sandboxsendertenantstate" ("tenant_name" varchar_pattern_ops);
COMMIT;

products/workflows/backend/migrations/0042_sandbox_sender_organization_block.py

2026-11-04T07:50:56.057678Z [error    ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=7061 tid=139793437633408
Traceback (most recent call last):
  File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
    geoip: Optional[GeoIP2] = GeoIP2(cache=8)
                              ~~~~~~^^^^^^^^^
  File "/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 114, in __init__
    raise GeoIP2Exception(
        "Path must be a valid database or directory containing databases."
    )
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
System check identified some issues:

WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model SandboxSenderOrganizationBlock
--
CREATE TABLE "workflows_sandboxsenderorganizationblock" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "reason" text NOT NULL, "created_at" timestamp with time zone NOT NULL, "created_by_id" integer NULL, "organization_id" uuid NOT NULL UNIQUE);
CREATE INDEX "workflows_sandboxsenderorganizationblock_created_by_id_81f4b1bd" ON "workflows_sandboxsenderorganizationblock" ("created_by_id");
COMMIT;

Last updated: 2026-11-04 07:51 UTC (9ce0beb)

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 2 Safe | 0 Needs Review | 0 Blocked

✅ Safe

Brief or no lock, backwards compatible

workflows.0041_sandbox_sender_tenant_state
  └─ #1 ✅ CreateModel
     Creating new table is safe
     model: SandboxSenderTenantState
  │
  └──> ℹ️  INFO:
       ℹ️  Skipped operations on newly created tables (empty tables
       don't cause lock contention).
workflows.0042_sandbox_sender_organization_block
  └─ #1 ✅ CreateModel
     Creating new table is safe
     model: SandboxSenderOrganizationBlock
  │
  └──> ℹ️  INFO:
       ℹ️  Skipped operations on newly created tables (empty tables
       don't cause lock contention).

Last updated: 2026-11-04 07:51 UTC (9ce0beb)

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed bbf56be: 3 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.

return TeamSendingHistory(
team_id=team_id,
sent=sum(counts.get(SENT_METRIC, 0) for counts in days.values()),
sent=sum(own_sends(counts) for counts in days.values()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 · Floor sandbox subtraction per source before summing daily sends

When a workflow has unmatched email_sandbox_sent metrics for a day, fetch_app_metric_daily_totals_by_team merges them with other workflows before own_sends applies its floor. Those sandbox counts then subtract another workflow's real sends, understating promotion volume and distorting reputation rates. Subtract and floor per source and day before summing team totals.

Comment on lines +92 to +94
with transaction.atomic():
state = SandboxSenderTenantState.objects.select_for_update().get(tenant_name=tenant_name)
tenant = (provider or SESProvider()).get_tenant_sending_state(tenant_name)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 · Bound SES calls and lock waits in the sandbox sync

During an SES slowdown, sync_sandbox_tenant_state holds the shared row lock across two API calls using boto3's default 60-second timeouts and retries. Concurrent webhook tasks block on that lock while occupying DEFAULT queue workers and database connections, delaying unrelated work. Bound the SES calls and use nonblocking locking with retry or coalescing.


def reload_sandbox_sender_state_on_workers(tenant_name: str) -> None:
logger.info(f"Reloading sandbox sender state for tenant {tenant_name} on workers")
publish_message("reload-sandbox-sender-state", {"tenantName": tenant_name})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 · Wire the sandbox reload message to a worker consumer

When reconciliation records DISABLED, this publishes to a channel that no worker subscribes to. Workers also never read SandboxSenderTenantState; SandboxEmailSender.canSend depends only on configuration, so they continue attempting sends against the paused tenant. Add a worker state reader and subscriber, and enforce that state before sending.

Silthus and others added 5 commits November 3, 2026 17:40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…afety/django

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/django

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…afety/django

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus

Silthus commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Split into smaller PRs in five stacks: Node caps #129048 to #129052, Node readers #129053 to #129057, product backend #129058 to #129062, editor #129063 to #129067, and the switch banner #129068 to #129069.

@Silthus Silthus closed this Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant